Documentation is needed for this

@anthony-c-martin does anyone on the team have time for a review? 😃

@johnlokerse does it works for types as well? Would be very good if it applies to types as well or if there is separate rule for it. There is one exception for types though - discriminator. for the example below it should not apply as the description for foo is where foo is referenced. If there is description directly on the type it does not appear anywhere.

...
@discriminator('type)
type foo = foo1 | foo2
...

@johnlokerse does it works for types as well? Would be very good if it applies to types as well or if there is separate rule for it. There is one exception for types though - discriminator. for the example below it should not apply as the description for foo is where foo is referenced. If there is description directly on the type it does not appear anywhere.

...
@discriminator('type)
type foo = foo1 | foo2
...

@slavizh This can be done. I would do it separately from this linter, though, since not everyone wants to put descriptions on properties in types or on the type itself. The same thing applies to variables and outputs if you ask me.

Or as another option we introduce a configuration to the new use-descriptions linter rule, just like use-recent-api-versions has with the maxAllowedAgeInDays configuration in the bicepconfig.json. Like so:

	"analyzers": {
		"core": {
			"rules": {
				"use-recent-api-versions": {
					"level": "warning",
					"maxAllowedAgeInDays": 500
				},
				"use-descriptions": {
					"level": "error",
					"variables": false,
					"userDefinedTypes": true,
					"parameters": true,
					"outputs": true
				}
			}
		}
	}

Or as another option we introduce a configuration to the new use-descriptions linter rule, just like use-recent-api-versions has with the maxAllowedAgeInDays configuration in the bicepconfig.json. Like so:

@johnlokerse - the changes in this PR look good to me, but I think it would be a good idea to close on this discussion first - I wouldn't want to release something and then immediately change it.

Or as another option we introduce a configuration to the new use-descriptions linter rule, just like use-recent-api-versions has with the maxAllowedAgeInDays configuration in the bicepconfig.json. Like so:

@johnlokerse - the changes in this PR look good to me, but I think it would be a good idea to close on this discussion first - I wouldn't want to release something and then immediately change it.

I agree.

@anthony-c-martin @slavizh What is your take on the proposed configuration in the bicepconfig.json? If you think it's good, I would suggest setting the configurable option (= variables, parameters, udt and output bool toggles) to true if not set in the bicepconfig else use the user-defined value set in the bicepconfig.

@anthony-c-martin @slavizh What is your take on the proposed configuration in the bicepconfig.json? If you think it's good, I would suggest setting the configurable option (= variables, parameters, udt and output bool toggles) to true if not set in the bicepconfig else use the user-defined value set in the bicepconfig.

What would the "default" behavior be - just parameters?

Another option to consider for how you express this in config is with an array of enums - e.g.:

"use-descriptions": {
  "level": "error",
  "requireOn": ["parameter", "type", "typeProperty", "output"]
}

@anthony-c-martin looks good to me. For parameters by default the highest could be only warning otherwise it will be a breaking change. I am ok to be a warning instead of off. For the others from my perspective they should be warnings by default but I think that might be too aggressive for many users and I do not know if they will react positively on it. Note that the exception on discriminator type is important as that description does not really appear anywhere when it is standalone type referenced.

@anthony-c-martin Looks good to me too, and I like the requiredOn property with the array values. However, if we look at the current list of linter rules, this introduces an inconsistency with how the other linter rules work:

If we choose the same approach as the no-unused-* rules, it would be use-description-* instead of configuring it in bicepconfig.json via requiredOn. This would be consistent with the other linter rules. What do you think?

@slavizh I agree on the warning default for parameters, but not for the others. I don’t see many organisations using descriptions for anything besides parameters. I recommend turning these off by default.

@johnlokerse note my suggestion is not based on what is popular but rather what should be the best practice. For example you will see most folks using multiple parameters trying to implement different features for different services that are deployed within the same module instead of trying to group those into a single parameter per resource. Very small example:

param resource1Name string
param resource1Sku string
param resource1MinimumTlsVersion string

param resource2Name string
param resource2MinimumTlsVersion string
param resource2PublicNetworkAccess string

vs.

type resource1Type = {
  name: string
  sku: string
  minimumTlsVersion: string
}

type resource2Type = {
  name: string
  publicNetworkAccess: string
  minimumTlsVersion: string
}
param resource1 resource1Type

param resource2 resource2Type

The latter has a ton of benefits that makes the user experience of modules better. Unfortunately even AVD is implementing that partially. One of the reason for not being widely used is that most folks moved from ARM templates where we did not have such features and everything was flat. Another is that the Portal does not supports such types very well thus the examples created by Microsoft employees also have flat structure. However this is another topic. Overall I think the standard values should represent the desired standard for coding even if it is not what is widely used in order to promote that standard. I also know that some warnings might be annoying in simple scenarios.

@johnlokerse, I'm happy defaulting to "off" for everything, because of the sheer quantity of .bicep files this would flag if enabled by default.

If we choose the same approach as the no-unused-* rules, it would be use-description-* instead of configuring it in bicepconfig.json via requiredOn. This would be consistent with the other linter rules. What do you think?

Good point! I'm fine either way. I think there's benefit to the ease of enabling/disabling if we create separate linters for each.

@johnlokerse, I'm happy defaulting to "off" for everything, because of the sheer quantity of .bicep files this would flag if enabled by default.

If we choose the same approach as the no-unused-* rules, it would be use-description-* instead of configuring it in bicepconfig.json via requiredOn. This would be consistent with the other linter rules. What do you think?

Good point! I'm fine either way. I think there's benefit to the ease of enabling/disabling if we create separate linters for each.

@anthony-c-martin Okay, so let's keep it consistent with the other linter rules. We default to off, and the following linters will be implemented:

1. use-description-parameters
2. use-description-vars
3. use-description-type
4. use-description-type-property
5. use-description-output

Additionally, documentation needs to be created. Will use the same style as this page: https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/linter-rule-no-unused-parameters.

Do I miss anything? If not, I will start building and will update this PR with the new linter rules. 😃